Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[8.0] Added configurable chunksize option to dirac_dms_replicate_and_r… #7672

Merged
merged 1 commit into from
Jun 25, 2024

Conversation

marianne013
Copy link
Contributor

@marianne013 marianne013 commented Jun 12, 2024

…egister_request script

BEGINRELEASENOTES

*DataManagement
CHANGE: dirac-dms-replicate-and-register-request: Make request chunk size configurable; default behaviour unchanged.

ENDRELEASENOTES

@DIRACGridBot DIRACGridBot added the alsoTargeting:integration Cherry pick this PR to integration after merge label Jun 12, 2024
@marianne013 marianne013 changed the title new: Added configurable chunksize option to dirac_dms_replicate_and_r… feat: Added configurable chunksize option to dirac_dms_replicate_and_r… Jun 12, 2024
Copy link
Contributor

@andresailer andresailer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can afford the extra 'u's

@marianne013
Copy link
Contributor Author

marianne013 commented Jun 12, 2024

I left the 'u' out to make it clear that chunksize (line 36) is not the same as chnksize (line 37). At least not in programming terms. I don't do much programming, I need all the help I can get.
#featurenotbug

@andresailer
Copy link
Contributor

One is a "string" and the other a variable, no harm to use the identical characters.
The same is done for catalog. It is much harder to read misspelled text, and remember which misspelling was used.

@marianne013
Copy link
Contributor Author

marianne013 commented Jun 12, 2024

The "catalog" is not mine.
<digs heels in>
Took me some time to work out how to display the brackets.

@andresailer
Copy link
Contributor

Yeah, I know, you should do as the romans with the catalog.

@marianne013
Copy link
Contributor Author

I still think my way is better, and I can't even hit "commit suggestion" as it will then fail the "commit message formatting" test.

@@ -22,15 +22,19 @@ def getLFNList(arg):
def main():
catalog = None
Script.registerSwitch("C:", "Catalog=", "Catalog to use")
Script.registerSwitch("N:", "ChunkSize=", "Number of files per request")
Copy link
Contributor

@andresailer andresailer Jun 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Script.registerSwitch("N:", "ChunkSize=", "Number of files per request")
Script.registerSwitch("N:", "ChnkSze=", "Number of files per request")

To not confuse it with line 36 and 37!

Edit: /s

@andresailer
Copy link
Contributor

I can't even hit "commit suggestion" as it will then fail the "commit message formatting" test.

You can "add suggestion to batch" and then type your own commit message.
https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/incorporating-feedback-in-your-pull-request

@fstagni fstagni changed the title feat: Added configurable chunksize option to dirac_dms_replicate_and_r… [8.0] Added configurable chunksize option to dirac_dms_replicate_and_r… Jun 18, 2024
@marianne013
Copy link
Contributor Author

I've added various 'u'. Everyone happy ?

@fstagni fstagni merged commit 4ff10ef into DIRACGrid:rel-v8r0 Jun 25, 2024
26 checks passed
@DIRACGridBot DIRACGridBot added the sweep:done All sweeping actions have been done for this PR label Jun 25, 2024
DIRACGridBot pushed a commit to DIRACGridBot/DIRAC that referenced this pull request Jun 25, 2024
@DIRACGridBot
Copy link

Sweep summary

Sweep ran in https://github.com/DIRACGrid/DIRAC/actions/runs/9664655301

Successful:

  • integration

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
alsoTargeting:integration Cherry pick this PR to integration after merge sweep:done All sweeping actions have been done for this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants